Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update typescript interface function definion and bump version to 2.0.0 #23

Merged
merged 8 commits into from
Aug 9, 2023

Conversation

v9n
Copy link
Member

@v9n v9n commented Aug 5, 2023

Follow Step 2 and 3

npm install
cd packages/api-augment
npm run clean:defs
npm run generate

then run:

npm run clean
npm run build

Note: I changed the port 8855 in the download-metadata script to my local 2.0.0 rpc node port 9946

@v9n v9n changed the base branch from main to update-test-for-new-taskid-refactor August 5, 2023 02:44
@v9n v9n requested review from imstar15 and chrisli30 and removed request for imstar15 August 5, 2023 02:46
Base automatically changed from update-test-for-new-taskid-refactor to main August 5, 2023 02:46
@v9n v9n changed the title Update interface ts def and bump version t0 2.0.0 Update typescript interface function definion and bump version to 2.0.0 Aug 5, 2023
Copy link
Member

@chrisli30 chrisli30 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I don’t think the events in augment-api-events.ts is updated correctly. Either there’s no logic to pick up the deleted event names, or the local node is still returning them through rpc.

@v9n maybe try to completely delete this file before running the npm scripts?

package.json Outdated
@@ -1,6 +1,6 @@
{
"name": "oak.js",
"version": "1.9.0-rc.4",
"version": "2.0.0-rc.4",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This version needs to be reset to "2.0.0-rc.1", meaning release candidate 1 for 2.0.0

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks. I just corrected it.

/**
* The result of the DynamicDispatch action.
**/
DynamicDispatchResult: AugmentedEvent<ApiType, [who: AccountId32, taskId: H256, result: Result<Null, SpRuntimeDispatchError>], { who: AccountId32, taskId: H256, result: Result<Null, SpRuntimeDispatchError> }>;
DynamicDispatchResult: AugmentedEvent<ApiType, [who: AccountId32, taskId: Bytes, result: Result<Null, SpRuntimeDispatchError>], { who: AccountId32, taskId: Bytes, result: Result<Null, SpRuntimeDispatchError> }>;
/**
* Notify event for the task.
**/
Notify: AugmentedEvent<ApiType, [message: Bytes], { message: Bytes }>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn’t this Notify in automationTime deleted?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the Notify event is still there to support other test. We don't expose the extrins etc but the event still in the Event enum. Let me try to remove them all from the blockchain.

/**
* Notify event for the task.
**/
Notify: AugmentedEvent<ApiType, [message: Bytes], { message: Bytes }>;
SuccesfullyAutoCompoundedDelegatorStake: AugmentedEvent<ApiType, [taskId: H256, amount: u128], { taskId: H256, amount: u128 }>;
SuccesfullyAutoCompoundedDelegatorStake: AugmentedEvent<ApiType, [taskId: Bytes, amount: u128], { taskId: Bytes, amount: u128 }>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be deleted.

@chrisli30
Copy link
Member

@imstar15, the types package’s code is behind. How do we update the types package?
CleanShot 2023-08-04 at 19 57 39

@imstar15
Copy link
Member

imstar15 commented Aug 5, 2023

@imstar15, the types package’s code is behind. How do we update the types package? CleanShot 2023-08-04 at 19 57 39

This line of configuration also needs to be manually updated to 2.0.0-rc.1.
packages/api-augment/package.json also needs to be manually updated to 2.0.0-rc.1.

package-lock.json Outdated Show resolved Hide resolved
chrisli30
chrisli30 previously approved these changes Aug 6, 2023
Copy link
Member

@chrisli30 chrisli30 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes are published on npm as v2.0.0-rc.1 as tested in xcm-demo code. Good to merge.

@v9n A couple of things to keep in mind of.

  1. The commit message regen is too simple. Please ALWAYS make sure the message explains the reason a code commit. For example, in the case you can use Regenerate the api-augment files with the correct oak-blockchain version v2.0.0
  2. The previous version of this PR was incorrect. I assumed that the regenerate scripts ran against an old blockchain code version.

@v9n
Copy link
Member Author

v9n commented Aug 6, 2023

@chrisli30 Sounds good. 👍 for the commit message. I will usually rebase and write more description commit before we merge.

@chrisli30 chrisli30 merged commit 78530a3 into main Aug 9, 2023
1 check passed
@chrisli30 chrisli30 deleted the update-interface-ts-def branch August 9, 2023 02:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants